Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open jobs #740

Merged
merged 10 commits into from
Aug 23, 2024
Merged

Open jobs #740

merged 10 commits into from
Aug 23, 2024

Conversation

spirali
Copy link
Collaborator

@spirali spirali commented Aug 16, 2024

This PR implemented open jobs.

NOT IMPLEMENTED YET: Dependencies between different submits into the same job. The server & scheduler is ready for this, mostly what is missing is to prepare an interface for submitting such tasks).

@spirali spirali requested a review from Kobzol August 16, 2024 16:39
Copy link
Collaborator

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! ❤️

And even though it was a 1k line commit as usual 😂 This time it was relatively easy to review :)

A few additional comments:

  • When I try to submit into a closed job, it crashes with a backtrace:
./hq job submit hostname
./hq job submit --job 1 hostname

Error: Received error: "Job 1 is not open"
<backtrace>

Same when a job is not found and when you attach an array job with a task ID that already exists. It should print some nicer error instead.

  • When a new submit is attached to an existing job, we should print a different message than this: Job submitted successfully, job ID: 2. It should say something like "New task(s) were attached to job 2".

CHANGELOG.md Outdated Show resolved Hide resolved
crates/hyperqueue/src/client/commands/job.rs Outdated Show resolved Hide resolved
crates/hyperqueue/src/client/commands/job.rs Outdated Show resolved Hide resolved
crates/hyperqueue/src/client/commands/submit/command.rs Outdated Show resolved Hide resolved
crates/hyperqueue/src/common/cli.rs Outdated Show resolved Hide resolved
docs/jobs/openjobs.md Outdated Show resolved Hide resolved
docs/jobs/openjobs.md Outdated Show resolved Hide resolved
docs/jobs/openjobs.md Outdated Show resolved Hide resolved
docs/jobs/openjobs.md Show resolved Hide resolved
docs/jobs/openjobs.md Outdated Show resolved Hide resolved
@spirali
Copy link
Collaborator Author

spirali commented Aug 19, 2024

  • Job attaching errors are now nicer
  • You can now use hq wait --without-close ... to wait only for tasks without necessity of closing the job

Copy link
Collaborator

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Left a few more nits.

crates/hyperqueue/src/common/cli.rs Outdated Show resolved Hide resolved
crates/hyperqueue/src/common/cli.rs Show resolved Hide resolved
crates/hyperqueue/src/server/job.rs Outdated Show resolved Hide resolved
tests/test_job.py Outdated Show resolved Hide resolved
docs/jobs/openjobs.md Outdated Show resolved Hide resolved
tests/test_job.py Outdated Show resolved Hide resolved
crates/hyperqueue/src/server/event/payload.rs Outdated Show resolved Hide resolved
crates/hyperqueue/src/client/output/cli.rs Show resolved Hide resolved
crates/hyperqueue/src/client/output/cli.rs Show resolved Hide resolved
crates/hyperqueue/src/bin/hq.rs Show resolved Hide resolved
@spirali
Copy link
Collaborator Author

spirali commented Aug 23, 2024

I think that all issues have been resolved.

Copy link
Collaborator

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, this is really cool.

@Kobzol Kobzol merged commit c2731c9 into main Aug 23, 2024
8 checks passed
@Kobzol Kobzol deleted the session branch August 23, 2024 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants